Skip to content

feat(bot): Merge new features from dev branch - #15

Merged
atlas-oc merged 29 commits into
mainfrom
atlas-dev
Feb 2, 2026
Merged

atlas-oc merged 29 commits into
mainfrom
atlas-dev

Conversation

@atlas-oc

@atlas-oc atlas-oc commented Jan 24, 2026 •

Copy link
Copy Markdown
Member

Notable changes:

  • Slash command autocomplete w/ cached trie structures
  • Add average shard latency to /bot-info
  • General code tidying
  • Use of macros to eliminate repetitive boilerplate
  • Some bug fixes

TODO:

  • Manual autocomplete cache invalidation for consistency
  • Add autocomplete for /front
  • Add autocomplete for /friend
  • Add tags to Discord (not yet sure if this will be a separate PR or not)

Check commits for full changelog.

Fixes OctoconDev/issues#355.
Fixes OctoconDev/issues#474.

@atlas-oc atlas-oc self-assigned this Jan 24, 2026
@atlas-oc atlas-oc added Enhancement Implementation of a new feature. Discord This is regarding the Octocon Discord bot. High Priority This is a high priority PR. labels Jan 24, 2026
@atlas-oc
atlas-oc merged commit 437db0d into main Feb 2, 2026
2 checks passed
Azyyyyyy referenced this pull request in Azyyyyyy/Interfold Jul 10, 2026
The three SimpleStatement construction sites in ScyllaEncryptionStateRepository
(:35 SELECT, :61 UPDATE-no-salt, :70 UPDATE-with-salt) were passing the
SystemId struct directly into the Cassandra driver's `params object[]`
instead of unwrapping to .Value. The driver has no serializer for SystemId;
the struct is boxed and only reaches the correct wire representation because
`Object.ToString()` ends up being called by the driver's fallback path and
`SystemId.ToString()` happens to equal `.Value` today - but that is not a
contractual guarantee of the wrapper. A future "safer" `ToString` variant
that redacted (as `SocketToken`, `Jti`, `LinkToken`, `RecoveryCode`,
`PushToken`, `ImportToken` all do) would silently break every
encryption-state lookup and write with no compile error and no runtime
exception, just wrong-key CQL - the classic silent-misbind failure mode.

Every sibling Scylla repo already binds `.Value` at the driver boundary -
`ScyllaNotificationTokenRepository:37,40` and `ScyllaSharedQueries:40-41`
are the reference shape - so this call site was drift, not intent. Fix
restores the same idiom and adds an inline comment explaining why the
unwrap must stay.

Round-2 strong-typing rescan Commit 1/14 (Finding #15, HIGH severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

Api.UnitTests 214/214, Bootstrapper.UnitTests 375/375 (1 platform skip),
build clean (0 errors, warnings unchanged from Round-4 baseline).

Co-authored-by: Cursor <cursoragent@cursor.com>
Azyyyyyy referenced this pull request in Azyyyyyy/Interfold Jul 16, 2026
* Fix options consistently drifting and make comments be shorter

* The start of the big cleanup, starting with strong typing

* Round 2 of strong typing

* Round 3 of strong typing

* Round 4 - Infra will be next

* Round 1 for Infa

* Round 2 for Infa

* Round 3 for Infa

* Round 4 - many fixes required

* Fix WebSocket join gate to tolerate scoped-sub JWTs on raw topics

* Fix more issues from infra changes

* Round 4 of strong typing - Steps 1-7 (plan complete)

Ships all 7 steps of the strong-typing rescan plan from
canvases/strong-typing-rescan.canvas.tsx.

- Step 1: SocketToken wired through WebSocketHandler; ToString redacts the
  JWT so any incidental log-interpolation along the six-hop socket path
  cannot leak the credential. Explicit .Value survives only at the two
  survival points that need the raw JWT (JwtSecurityTokenHandler and the
  Authorization: Bearer header write).
- Step 2: IScyllaKeyspaceResolver.NormalizeSystemId / ResolveRegionalKeyspace
  retyped as SystemId at the interface; 12 Scylla repos + ScyllaSharedQueries
  updated. SystemId.Value continues to be the driver boundary; the
  extension-method convenience overloads are gone.
- Step 3: IRegionContext.ResolveUserRegion(string) removed; only the SystemId
  overload survives. InMemory + Scylla implementations aligned; Scylla keeps
  a private string-typed helper so its sibling public methods can share
  logic without leaking string back through the interface.
- Step 4: SystemId? kept typed end-to-end through the WebSocketHandler
  socket-join gate (:166, :190, :591, :661) - no more raw-string hops.
- Step 5: WebSocket JWT sub parsed via ScopedSystemId.TryParseScoped inside
  IsSocketJoinTokenAuthorizedAsync; middleware parity for HTTP vs socket
  paths is now type-enforced rather than reimplemented via string.Equals.
- Step 6: RecoveryCodeResolver.TryResolve out-param retyped from string to
  RecoveryCode. Plaintext exists as a bare string only inside the private
  decrypt frame (which has no logging surface); every external observation
  goes through the redacting wrapper. SettingsController SetupEncryption /
  RecoverEncryption / ImportSp collapse to pass the wrapper directly.
- Step 7: Jti.From(string?) / LinkToken.From(string?) static factories added
  to SecretTokens.cs (return null for null/empty/whitespace, wrap otherwise
  - total function, not TryParse-style). AuthController.RevokeToken and
  AuthLinkController.Callback null-check on the typed nullable so the raw
  string no longer lives across the guard/wrap boundary.

New test files:
- SocketTokenRedactionTests: pins ToString-redacts and .Value-unwraps-verbatim
  contracts that WebSocketHandler now leans on.
- SecretTokensFromFactoryTests: 8 tests, 4-input matrix + interpolation
  round-trip for both Jti.From and LinkToken.From.

Modified tests:
- IsTokenSubjectAuthorizedForTopicTests: reduced 13 -> 7 tests as raw-sub
  scenarios became compile-time-impossible after Step 5. Rejection matrix
  migrated to ScopedSystemIdTests where it belongs.
- InMemoryRegionContextTests, InMemoryAccountRepositoryLinkTokenRegressionTests,
  RegionContextCachingTests, ScyllaFrontingRepositoryNullTimeStartTests: all
  retyped to speak SystemId at the boundaries the interface now demands.

Api.UnitTests 214/214, Bootstrapper.UnitTests 375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix Scylla misbind: bind SystemId.Value in encryption state repo

The three SimpleStatement construction sites in ScyllaEncryptionStateRepository
(:35 SELECT, :61 UPDATE-no-salt, :70 UPDATE-with-salt) were passing the
SystemId struct directly into the Cassandra driver's `params object[]`
instead of unwrapping to .Value. The driver has no serializer for SystemId;
the struct is boxed and only reaches the correct wire representation because
`Object.ToString()` ends up being called by the driver's fallback path and
`SystemId.ToString()` happens to equal `.Value` today - but that is not a
contractual guarantee of the wrapper. A future "safer" `ToString` variant
that redacted (as `SocketToken`, `Jti`, `LinkToken`, `RecoveryCode`,
`PushToken`, `ImportToken` all do) would silently break every
encryption-state lookup and write with no compile error and no runtime
exception, just wrong-key CQL - the classic silent-misbind failure mode.

Every sibling Scylla repo already binds `.Value` at the driver boundary -
`ScyllaNotificationTokenRepository:37,40` and `ScyllaSharedQueries:40-41`
are the reference shape - so this call site was drift, not intent. Fix
restores the same idiom and adds an inline comment explaining why the
unwrap must stay.

Round-2 strong-typing rescan Commit 1/14 (Finding #15, HIGH severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

Api.UnitTests 214/214, Bootstrapper.UnitTests 375/375 (1 platform skip),
build clean (0 errors, warnings unchanged from Round-4 baseline).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Retype IAvatarStorage to speak AvatarUrl end-to-end

Round-2 strong-typing rescan Commit 2/14 (Findings #1, #2, #8, HIGH severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

IAvatarStorage previously returned raw strings from the two save methods and
took `string? avatarUrl` on the delete method, so every caller was doing
`new AvatarUrl(url)` on the very next line and every delete-site was doing
`currentAvatarUrl?.Value` to strip back to string right before the call.
The AvatarUrl wrapper (and its `FromNullable` factory) has existed the whole
time - this commit rips out the pointless round-trip.

Interface + LocalAvatarStorage:
- `SaveSystemAvatarAsync` / `SaveAlterAvatarAsync` return `Task<AvatarUrl>`.
- `DeleteByUrlAsync` takes `AvatarUrl?` and pattern-matches to extract
  `.Value` at the filesystem boundary (still needed for URI parsing).
- Private helper `SaveAsync` retyped from `(string systemId, string targetId
  ...)` to `(SystemId systemId, string targetId, ...)` so callers stop
  pre-unwrapping .Value only for the helper to re-normalize immediately
  (Finding #8). targetId legitimately polymorphs between "self" (system
  avatar) and alterId.ToString() (alter avatar), so it stays a string
  segment - no wrapper covers both shapes. Wrap of the final URL happens
  once in the private helper's return statement.

Callers - production:
- SettingsController: 1 save (:290) + 3 delete sites (:332, :392, :424).
  `string avatarUrl` local retyped to `AvatarUrl`; `new AvatarUrl(avatarUrl)`
  at the CommandEnvelope payload collapses to just `avatarUrl`; every
  DeleteByUrlAsync call passes `currentAvatarUrl` (AvatarUrl?) directly.
- AltersController: 1 save (:166) + 3 delete sites (:226, :304, :417).
  Same collapse pattern.
- SimplyPluralImportService: 2 saves (:850 system, :857 alter). Removes
  `new AvatarUrl(localUrl)` at :851 and `AvatarUrl.FromNullable(localUrl)`
  at :861 - localUrl is now AvatarUrl and widens implicitly to AvatarUrl?
  at the alter-command payload.

Callers - test/util mocks (interface signature must match):
- SPDump/Program.cs TempAvatarStorage: three signatures aligned; empty-URL
  returns wrap `new AvatarUrl("")` (AvatarUrl ctor accepts empty, only null
  throws - matches existing behaviour of returning `""` before).
- IntegrationTests/SimplyPluralImport/SpImportTests.cs NullAvatarStorage:
  three signatures aligned identically.

Build clean (0 errors, 100 warnings - unchanged from Round-1 baseline).
Api.UnitTests 214/214, Bootstrapper.UnitTests 375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Retype EncryptionKey.DeriveKey / DeriveChecksum to speak wrappers

Round-2 strong-typing rescan Commit 3/14 (Findings #12 + #13 + #14, HIGH severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

EncryptionKey.DeriveKey and DeriveChecksum were the funnel through which every
strongly-typed value in the encryption pipeline was stripped back to raw
string. The two handler call sites each shipped a `.Value.Value.Value` unwrap
ladder and every command result re-wrapped the derived key back into
`EncryptionKeyMaterial` on the next line. This commit promotes the API so
the wrappers survive the KDF/checksum round-trip end-to-end.

API changes (Interfold.Domain/EncryptionKey.cs):
- `DeriveKey(string pepper, SystemId systemId, RecoveryCode recoveryCode,
   EncryptionSalt salt): EncryptionKeyMaterial` - three of four inputs and
   the return value moved from string to their existing wrappers. Only the
   pepper stays raw because no wrapper exists for deployment-wide crypto
   secrets today (creating one would only force an unnecessary .Value at
   every use).
- `DeriveChecksum(EncryptionKeyMaterial key): KeyChecksum` - both directions
   typed, so the handler's checksum-equality check no longer needs
   `string.Equals(..., Ordinal)` - `KeyChecksum` is a `readonly record
   struct` whose auto-generated Equals compares .Value with ordinal string
   equality (default `EqualityComparer<string>`), so `checksum != keyChecksum`
   is semantically identical to the old comparison.

Call-site cleanup:
- SetupEncryptionCommandHandler (:70-77): three .Value strips and two
  wrapper re-constructions (`new KeyChecksum(checksum)`, `new
  EncryptionKeyMaterial(key)`) collapse to pass wrappers directly.
- RecoverEncryptionCommandHandler (:65-71): same three .Value strips + one
  re-construction collapse; string.Equals replaced by record-struct `!=`.
- WireByteFreezeTests.DeriveKey_IsDeterministic_AndScopeSensitive: string
  literals wrapped at the call boundary. The scoped-vs-raw byte-freeze
  assertion still passes byte-for-byte - the wire form now enforced by the
  wrapper types themselves rather than by convention among callers.

Bridge (removed in Commit 5):
- SimplyPluralImportService.ValidateEncryptionKeyAsync (:758) still takes
  `string recoveryCode` because Finding #3 (Commit 5) retypes that method
  end-to-end. Bridge inline-wraps `new RecoveryCode(recoveryCode)` at the
  call and unwraps `key.Value` at the tuple return; both go away in Commit
  5 when the method signature and its two callers flip.

Build clean (0 errors, 100 warnings - unchanged from Commit 2 baseline).
Api.UnitTests 214/214 (WireByteFreezeTests all green -
DeriveKey_IsDeterministic_AndScopeSensitive still pins the region-prefix
sensitivity byte-for-byte), Bootstrapper.UnitTests 375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Add Jti.NewJti() mint factory and adopt at IssueDeepLinkTokenAsync + socket revoke check

Round-2 strong-typing rescan Commit 4/14 (Findings #7 + #11, MEDIUM/LOW severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

Two Step-7-adjacent raw-string-local windows kept the JTI alive as an
interpolable string across a mint/wrap or read/wrap boundary. Round-2 Commit 4
closes both by adding a `Jti.NewJti()` mint factory and adopting the existing
`Jti.From(string?)` at the socket-side read site.

New factory (Interfold.Contracts/Ids/SecretTokens.cs):
- `Jti.NewJti()`: mints a fresh `Guid.NewGuid().ToString("N")` and wraps in a
  single call. Byte-identical to the pre-Round-2 raw shape (32-char lowercase
  hex, no dashes) so every pre-Round-2 issued token still verifies against a
  Jti minted today and the revocation-store row key is unchanged. Total
  function - never returns default.

Adoption:
- AuthController.IssueDeepLinkTokenAsync (:169): raw string local
  `Guid.NewGuid().ToString("N")` replaced by `Jti.NewJti()`. Removes the two
  `new Jti(jti)` re-wraps at the CreateToken and RecordTokenAsync call sites.
- WebSocketHandler socket-join JTI-revocation check (:694-705): raw string
  local from `principal.FindFirstValue(JwtClaimNames.Jti)` replaced by
  `Jti.From(...)`; null check now runs on `Jti?` via `is { } typedJti`
  pattern-match. Removes the `new Jti(jti)` re-wrap at ValidateTokenNotRevoked
  and - more importantly - the LogWarning at :702 previously interpolated a
  bare string local (`{Jti}`) so the structured-log sink emitted the raw JTI
  verbatim on every revoked-token socket-join attempt; the typed local now
  routes through `Jti.ToString` which redacts.

Tests (Interfold.Api.UnitTests/Ids/SecretTokensFromFactoryTests.cs):
Three new tests added alongside the existing eight `From` tests:
- `JtiNewJti_ProducesUniqueValueEachCall`: two consecutive mints must differ.
  A collision would silently merge two independent revocation entries so
  revoking one would revoke both - the exact silent-security-failure this
  factory is designed to prevent.
- `JtiNewJti_ProducesThirtyTwoCharLowercaseHex`: pins the wire format
  byte-for-byte with the pre-Round-2 raw `Guid.NewGuid().ToString("N")`
  shape. Regex-anchors `^[0-9a-f]{32}$`. A drift would orphan every
  pre-Round-2 issued token on the first NewJti() issuance after deploy.
- `JtiNewJti_ValueRedactsUnderInterpolation`: `$"{jti}"` must not contain the
  raw value. Pins the AuthController.IssueDeepLinkTokenAsync safety story
  end-to-end - operation-id / redirect logs live in the same span as the
  mint and must not leak.

Build clean (0 errors, 100 warnings - unchanged from Commit 3 baseline).
Api.UnitTests 217/217 (+3 from Commit 3's 214), Bootstrapper.UnitTests
375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Retype SimplyPluralImportService recovery-code + encryption-key end-to-end

Round-2 strong-typing rescan Commit 5/14 (Findings #3 + #4, HIGH severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

The SP import service was the last consumer of the pre-Commit-3 raw-string
EncryptionKey API. Commit 3 shipped a bridge here (inline
`new RecoveryCode(recoveryCode)` at the DeriveKey call, `.Value` unwrap at the
tuple return) so the API flip could land in isolation. This commit removes
that bridge and retypes all six SP-side signatures so the recovery code and
derived key never appear as bare strings inside the import pipeline.

Six signatures / locals updated in SimplyPluralImportService.cs:

1. `ImportAsync` top-level local (:92): `string? encryptionKey` retyped to
   `EncryptionKeyMaterial?`. The "no derived key" case now encoded by null
   rather than an empty-string sentinel; the `IsNullOrWhiteSpace` gate at
   :131 becomes a `is { } derivedEncryptionKey` pattern-match.
2. `ImportAsync` -> `ValidateEncryptionKeyAsync` call site (:95): drops the
   `.Value` on providedRecoveryKey; passes the `RecoveryCode` wrapper.
3. `ValidateEncryptionKeyAsync` signature (:763): `string recoveryCode`
   retyped to `RecoveryCode`; return tuple's `string DerivedKey` retyped to
   `EncryptionKeyMaterial?` (null replaces `string.Empty` as the failure
   sentinel). The Commit-3 inline-wrap bridge (`new RecoveryCode(recoveryCode)`)
   is deleted; DeriveKey is called with the typed value directly.
4. `ImportNotesAsync` signature (:636): `string encryptionKey` retyped to
   `EncryptionKeyMaterial`.
5. `PrepareNote` signature (:703): `string encryptionKey` retyped to
   `EncryptionKeyMaterial`.
6. `TryEncryptForClient` signature (:730): `string base64Key` retyped to
   `EncryptionKeyMaterial`. The single `.Value` unwrap survives at the
   `Convert.FromBase64String` call - the base64-string form is the API
   contract of the crypto primitive, so this is a legitimate driver-boundary
   unwrap (same shape as `SystemId.Value` at the Cassandra bind boundary).

Wire behaviour: byte-identical. `EncryptionKeyMaterial` is a
`readonly record struct` whose `.Value` is the raw base64 string, so every
downstream AES-GCM encryption produces the identical ciphertext the pre-Commit-5
code produced for the same recovery code + salt + pepper triplet. The
byte-freeze test in `WireByteFreezeTests` (updated in Commit 3) transitively
covers the derivation input; the SP import ciphertext is unchanged because
the underlying key material bytes are unchanged.

Build clean (0 errors, 100 warnings - unchanged from Commit 4 baseline).
Api.UnitTests 217/217, Bootstrapper.UnitTests 375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Delete three ResolveFriendshipLevelAsync private wrappers - inline direct call

Round-2 strong-typing rescan Commit 6/14 (Findings #16 + #17, LOW severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

Three Scylla repositories (Alter, Tag, Fronting) each carried a byte-identical
private wrapper whose sole job was to hop the owner's SystemId string ->
typed at the ScyllaSharedQueries.ResolveFriendshipLevelAsync boundary:

    private Task<FriendshipLevel?> ResolveFriendshipLevelAsync(ISession session,
                                                                string ownerSystemId,
                                                                SystemId? viewerSystemId)
        => ScyllaSharedQueries.ResolveFriendshipLevelAsync(
                session, _keyspaceResolver,
                new SystemId(ownerSystemId),  // <- pure string-obsession
                viewerSystemId);

Every caller already held a typed `normalizedSystemId` local (result of
`_keyspaceResolver.NormalizeSystemId(systemId)` a few lines up) and was
passing `normalizedSystemId.Value` into the wrapper only to have the wrapper
immediately `new SystemId(...)` it back. The whole scaffolding existed only
because ScyllaSharedQueries was originally hand-rolled with `string` and the
callers were retrofitted to typed later without cleaning up the bridge.

Fix - one commit:
- Deleted the three private `ResolveFriendshipLevelAsync` wrappers from
  ScyllaFrontingRepository (:701-702), ScyllaAlterRepository (:587-588),
  and ScyllaTagRepository (:668-669).
- Updated all five call sites (:295 Fronting, :452/:528 Alter, :476/:579
  Tag) to call `ScyllaSharedQueries.ResolveFriendshipLevelAsync` directly
  with the typed `normalizedSystemId` and the already-injected
  `_keyspaceResolver`.

The InMemory-side sibling methods (`InMemoryAlterRepository.
ResolveFriendshipLevelAsync` and equivalents) are legitimate private helpers
with different signatures (system + viewer, no session, hits the in-memory
friendships dict directly). They are not the same anti-pattern and stay.

Build clean (0 errors, 100 warnings - unchanged from Commit 5 baseline).
Api.UnitTests 217/217, Bootstrapper.UnitTests 375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Introduce ProviderIdentity union - collapse 3 OAuth-provider re-wrap switches

Round-2 strong-typing rescan Commit 7/14 (Findings #5 + #6, MEDIUM severity).
Canvas: canvases/strong-typing-rescan-round-2.canvas.tsx.

The OAuth callback pipeline had three duplicated `oauthProvider switch`
statements, each of which independently re-wrapped the exact same raw
`identity` string in the exact same three ways:

    // AuthController.Callback:114
    var resolvedSystemId = oauthProvider switch
    {
        OAuthProvider.Discord => await _accounts.FindOrCreateSystemIdByDiscordIdAsync(new DiscordId(identity), ...),
        OAuthProvider.Google  => await _accounts.FindSystemIdByEmailAsync(new Email(identity), ...),
        OAuthProvider.Apple   => await _accounts.FindSystemIdByAppleIdAsync(new AppleId(identity), ...),
        _ => null,
    };

    // AuthLinkController.Callback:119 - same three wraps, different account API
    // AuthLinkController.RedirectWithSocketEventAsync:154 - same three wraps, different event type

The root of the smell was `ExtractProviderIdentityAsync` returning
`string?` even though its three sources (`DiscordOAuthService.
ExchangeCodeForDiscordIdAsync`, `GoogleOAuthService.ExchangeCodeForEmailAsync`,
`AppleOAuthService.ExchangeCodeForAppleIdAsync`) each already return the typed
nullable (`DiscordId?` / `Email?` / `AppleId?`). The wrapper was doing a
typed-to-string-to-typed round-trip and inviting every downstream site to
switch on the enum and rewrap manually.

New type (Interfold.Contracts/Ids/ProviderIdentity.cs):
- `ProviderIdentity` - a `readonly record struct` union with three nullable
  properties `Discord`, `Google`, `Apple`. Construction only via the three
  static factories `FromDiscord` / `FromGoogle` / `FromApple` that enforce
  the "exactly one populated" invariant at compile time.
- Callers dispatch via property-pattern match:
    identity switch
    {
        { Discord: { } discordId } => ...,
        { Google: { } email }      => ...,
        { Apple: { } appleId }     => ...,
        _ => (unreachable given caller-side presence guard)
    }

API changes:
- `OAuthControllerBase.ExtractProviderIdentityAsync`: return type flipped
  from `Task<string?>` to `Task<ProviderIdentity?>`. Inner logic
  reorganised so each provider branch builds up the typed value and wraps
  into the union at the return points (all fallback paths still hit the
  query/form untyped extraction and wrap at the end).
- `AuthController.Callback`: `string.IsNullOrWhiteSpace(identity)` guard
  replaced with `identity is not { } typedIdentity`; the account-lookup
  switch dispatches off the union rather than off `oauthProvider`.
- `AuthLinkController.Callback`: same guard + dispatch shape. The
  `oauthProvider switch` used for user-facing error text on AlreadyLinked /
  UserExists paths stays as-is (no string-wrap in those switches).
- `AuthLinkController.RedirectWithSocketEventAsync`: signature retyped
  from `(ScopedSystemId, OAuthProvider, string, string?)` to
  `(ScopedSystemId, ProviderIdentity, string?)`. Each PublishAsync call
  keeps its concrete event type so the event bus's generic subscriber
  routing continues to dispatch by TEvent as before.

Build clean (0 errors, 100 warnings - unchanged from Commit 6 baseline).
Api.UnitTests 217/217, Bootstrapper.UnitTests 375/375 (1 platform skip).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Introduce ScopedSystemId.RepresentsSameUserAs — replace 8 cross-type self-check byte compares

Round-2 Commit 8 (canvas finding #9). Fixes a latent semantic drift in the eight
controller self-request guards:

  FriendRequestsController — Send (line 64, UsernameOrSystemId), Cancel (:87),
  Accept (:110), Reject (:133) [SystemId]
  FriendsController — Show (:43), Delete (:72), Trust/Untrust (:108) [SystemId]

Pre-Round-2 spelling used one of two shapes, both of which reduced to a byte
compare of the scoped composite ("nam:abcdefg") against the raw route value:

  - `principal.Value == id.Value` at the Send site — cross-wrapper raw string
    compare of ScopedSystemId.Value vs UsernameOrSystemId.Value.
  - `principal == id` at the seven SystemId sites — the implicit
    ScopedSystemId→SystemId widen made this `SystemId(scoped) == SystemId(raw)`,
    still a byte compare.

Both shapes caught the trivial scoped-route case (`/api/friends/nam:abcdefg`)
and silently missed the raw-route case (`/api/friends/abcdefg`). For the
FriendRequestsController.Send site the Slice-7 documentation on
SendFriendRequestCommandHandler:65-73 correctly noted the downstream resolved-id
self-check picked up the miss, so Send only returned the wrong error code for
the fast path. The seven SystemId sites have NO downstream self-check — a raw
self-cancel / self-accept / self-reject / self-delete / self-trust silently fell
through to the domain handler and returned a generic
friend_request:not_requested or friendship_not_found instead of the crisp
cannot_*_self.

Fix: introduce a semantic primitive on ScopedSystemId and route all eight sites
through it.

  ScopedSystemId.RepresentsSameUserAs(SystemId candidate)
    - TryParseScoped(candidate.Value) → compare scoped composites (cross-region
      shapes are treated as different users per the "scoped composite is
      identity" contract on the type's xml-doc).
    - Else compare against principal.RawId (raw shape belongs to principal's
      region by convention — this is the case pre-Round-2 silently missed).
    - Blank / whitespace → false (never throws on defensive inputs).

  ScopedSystemId.RepresentsSameUserAs(UsernameOrSystemId candidate)
    - Delegates to the SystemId primitive when LookupHandle.TryParse returns
      LookupKind.Region or LookupKind.Id (scoped and bare raw shapes).
    - Username / Discord shapes return false — the controller can't decide
      "am I alice?" without a registry lookup, so the fast-path must let the
      downstream resolver's self-check take over (preserves Slice-7 semantics).
    - Unparseable prefixes (e.g. "xxx:abcdefg") fall back to a byte compare
      against Value so an opaque input still catches the trivial case.

Behaviour delta: the 7 SystemId sites now return the specific cannot_*_self
error code for the raw-route shape. The HTTP status is unchanged (400), so
clients that switch on status don't observe a difference; clients that switch
on error codes get a more precise category. The Send site's fast-path shape is
unchanged (byte-compat for the scoped-route case; the newly-caught raw-route
case is a strict improvement because the downstream handler was already going
to catch and reject the same call).

Audit outcome for the canvas note "audit AltersController / TagsController for
the same idiom": no `principal == id` shape in either controller — cleared.

Tests: Interfold.Api.UnitTests/Ids/ScopedSystemIdRepresentsSameUserAsTests.cs
adds 11 tests covering same-region-scoped, raw, cross-region-scoped, mismatched
raw, blank, and the four UsernameOrSystemId shapes (scoped, raw, username,
Discord, unknown prefix). All 228 API unit tests pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Retype ScyllaAccountRepository.LinkTokenEntry.Scoped + InMemoryAccountRepository.EnsureEncryptionSaltForSystem

Round-2 Commit 9 (canvas findings #18 + #21). Two small focused string-leak
closures at the boundary between the account-repository FindOrCreate paths and
their internal state.

#18 — ScyllaAccountRepository.LinkTokenEntry
    Retyped the record-struct's value field from `string ScopedSystemId` to
    the typed `ScopedSystemId Scoped`. Pre-Round-2 the Scylla adapter was the
    string-leak outlier — the InMemory sibling has always carried the typed
    shape. The reverse-map read at ResolveSystemIdByLinkTokenAsync no longer
    re-wraps through `new SystemId(entry.ScopedSystemId)` — it widens through
    the wrapper's AsSystemId instead.

    Cache-rehydration audit (per canvas note): confirmed
    _linkTokenBySystem / _systemByLinkToken are process-lifetime only. The
    ctor only captures the session provider / keyspace resolver / options; no
    load-from-Scylla seeding path. Safe to change the entry's value shape.

    Dictionary key/value types deliberately UNCHANGED — canvas findings #20 +
    #22 are scoped to Commit 12, which retypes the reverse-map keys to
    LinkToken and the forward-map values to LinkToken / ScopedSystemId.
    Keeping the boundaries split so if Commit 12 needs a follow-up, this
    smaller change stands on its own.

#21 — InMemoryAccountRepository.EnsureEncryptionSaltForSystem
    Retyped the parameter from `string scopedSystemId` to
    `ScopedSystemId scoped`. All three call sites (FindOrCreate for Discord /
    Email / Apple) already resolved a typed ScopedSystemId locally and had to
    unwrap to `.Value` at the boundary only for the method to re-wrap via
    `new SystemId(scopedSystemId)` inside the UpsertAsync call. The wrapper
    eliminates that round-trip and pins the "the caller already resolved this
    to a scoped composite" invariant at the type level.

    Each of the three FindOrCreate methods now holds a typed `scopedNew` local
    and returns `scopedNew.AsSystemId()` — removes the pre-Round-2
    `.Value` → `new SystemId(...)` round-trip on the return path as a side
    benefit. The `_systemByDiscord` / `_systemByEmail` / `_systemByApple`
    dicts remain string-valued (Commit 12 territory).

No behaviour change. Build passes with 100 warnings unchanged; all 228
Interfold.Api.UnitTests pass (includes InMemoryAccountRepositoryLinkTokenRegressionTests).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Delete untyped QualifyAvatar overloads — every consumer now speaks AvatarUrl?

Round-2 Commit 10 (canvas #10). Blanket-verified per the canvas note that
every consumer of QualifyAvatar migrated to the typed shape after Round-2
Commit 2 (AvatarStorage retype) promoted every read model and command payload
to AvatarUrl?.

Deletions:
  - AvatarUrlQualifier.QualifyAvatar(string? url, AvatarSource?, string scheme, HostString host)
  - AvatarUrlQualifier.QualifyAvatar(string? url, AvatarSource?, string? origin)
  - InterfoldControllerBase.QualifyAvatar(string? url, AvatarSource?)

The two typed AvatarUrlQualifier overloads used to delegate to the string
siblings via `AvatarUrl.FromNullable(QualifyAvatar(url?.Value, source, ...))`.
Post-Round-2 they own the AvatarSource discrimination directly and only reach
into the string primitive `Qualify` when the source is Local — one less
wrap/unwrap round-trip, and no possibility for a caller to accidentally bind
to the raw-string overload when the compiler had both to choose from.

The `AvatarUrlQualifier.Qualify(string?, ...)` overloads are retained — they
are the primitive URL-qualification helper used by non-avatar callers (and by
the typed `QualifyAvatar` internally). Only the AvatarSource-aware string
overloads went.

Verification:
  - Build clean, 100 warnings unchanged, 0 errors.
  - All 228 Interfold.Api.UnitTests pass.
  - Grep for `QualifyAvatar` returns only typed call sites in
    FriendsController / FriendRequestsController / AltersController /
    PublicSystemsController and the socket handlers
    (AlterSocketEventHandlers, FriendshipSocketEventHandlers,
    WebSocketInitialization). Zero string-typed consumers.

Also updated the AvatarServingPolicy xml-doc `<see cref>` to point at the
typed overload so the doc reference resolves post-deletion.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Extract ProviderColumn enum inside ScyllaAccountRepository — retype 4 internal helpers + 10 public entry points

Round-2 Commit 11 (canvas #19). Replaces the magic-string column discriminator
that the 10 public OAuth-provider entry points passed into 4 internal helpers.

Before: every public entry point (Try/FindOrCreateSystemIdBy{Discord,Email,Apple}Id
Async, Link{Discord,Email,Apple}ToUserAsync, Unlink{Discord,Email,Apple}Async)
hand-spelled the Cassandra column name as a literal — 10 sites all repeating
`"discord_id"`, `"email"`, or `"apple_id"`. The four internal helpers
(TryFindSystemIdByRegistryColumnAsync, FindOrCreateSystemIdByRegistryColumnAsync,
LinkIdentityAsync, UnlinkIdentityAsync) accepted `string columnName` and
interpolated it into every CQL fragment. A misspelled literal at any entry
point would compile silently and fail at runtime with a Cassandra "table does
not exist" or "unknown column" error — the exact class of drift the strong-
typing rescan is chasing out.

After: a private `ProviderColumn` enum (`Discord`, `Email`, `Apple`) is the
discriminator through every internal helper. A single `ColumnName(ProviderColumn)`
switch-with-explicit-throw maps the enum onto the CQL literal at the boundary
where each helper enters the SQL-string-interpolation layer. Misspelled provider
literals are now impossible (there's no string to misspell); a future
5th provider surfaces as build-time exhaustiveness warnings on the switch and
on every call site instead of a runtime silent fail.

The CQL strings themselves are unchanged, so wire format / storage schema /
integration-test fixtures are unaffected. Every one of the 10 entry points is
still a one-liner — the enum shifts the concern from "spell the column right"
to "pick the right enum value", which the type system can verify.

Verification: - Build clean (100 warnings unchanged, 0 errors).
  - All 228 Interfold.Api.UnitTests pass.
Co-authored-by: Cursor <cursoragent@cursor.com>

* round-2 commit 12: retype the account-repo dictionary state to speak wrappers

Round-2 finding #20 (ScyllaAccountRepository) + #22 (InMemoryAccountRepository)
of the second strong-typing rescan. Internal-only change to runtime cache
hygiene; zero interface / on-wire shape / storage-schema movement.

Scylla changes:

* `_linkTokenBySystem`  : `<string, string>`      -> `<ScopedSystemId, LinkToken>`
* `_systemByLinkToken`  : `<string, LinkTokenEntry>` -> `<LinkToken, LinkTokenEntry>`

The four `GetOrCreate` / `Get` / `Resolve` / `Clear` link-token methods now
hold the typed `ScopedSystemId` from `ScopedSystemId.Compose` directly rather
than immediately unwrapping to `.Value` for the dict key, and the reverse-map
lookup takes the typed `LinkToken` so no accidental `linkToken.ToString()`
call site can poke the dict (the wrapper redacts under `ToString`, which
would have silently corrupted the cache pre-Round-2).

InMemory changes (the wider one -- InMemory holds nine dicts, all typed):

* `_usernameBySystem`   value -> `Username`
* `_avatarBySystem`     value -> `AvatarUrl`
* `_linkTokenBySystem`  value -> `LinkToken`
* `_systemByLinkToken`  key   -> `LinkToken`
* `_discordBySystem`    value -> `DiscordId`
* `_emailBySystem`      value -> `Email`
* `_appleBySystem`      value -> `AppleId`
* `_systemByDiscord`    value -> `ScopedSystemId`
* `_systemByEmail`      value -> `ScopedSystemId`
* `_systemByApple`      value -> `ScopedSystemId`

`_descriptionBySystem` stays `<string, string>` because there is no
`Description` wrapper (descriptions are genuinely free-form user content,
not identifiers). The `_systemBy{Discord,Email,Apple}` dicts keep their
string-typed KEY with the pre-Round-2
`StringComparer.OrdinalIgnoreCase` comparer -- the identity wrappers do not
self-normalise, so the lookup contract is unchanged.

The `LinkIdentifier(...)` helper is promoted to a generic
`LinkIdentifier<TIdentity>(..., Func<TIdentity, string> extractRawValue)
where TIdentity : struct` so the three call sites (Discord / Email / Apple)
pass their typed identity and a static-lambda accessor for the reverse-map
key; static lambdas mean no per-call allocation. The three `Unlink*` /
`Delete` / `GetPublicProfile` methods now speak typed locals throughout, and
`GetPublicProfileAsync` no longer re-wraps each field with `new Username(u)`
/ `new DiscordId(discord)` etc. before feeding
`AccountPublicProfileReadModel` -- the dict values are already the right
shape.

Round-2 Commit 9 already routed `LinkTokenEntry.Scoped` through
`AsSystemId()` on the read side; Commit 12 finishes the job by ensuring the
scoped composite spends zero time as a bare string local on the write side
either.

Because the caches are process-lifetime only (audited in Commit 9 --
`ScyllaAccountRepository` reads no storage-side rehydration of either dict),
the key-shape / value-shape change is invisible externally. Every test run
that exercises the account repository (auth-link flows, avatar reads,
Discord OAuth callback, encryption-setup gate, and the SP-import fixture
that touches profile writes) is on the same wire and CQL bytes as it was
before this commit -- the only observable difference is that a compiler
error (rather than a `NullReferenceException` at runtime) now surfaces if
someone poke the wrong dict with the wrong wrapper type.

Build: 100 warnings (unchanged from Commit 11 baseline), 0 errors.
Api.UnitTests: 228 passed, 0 failed. No new tests -- Commit 12 is a
representation change with no new observable behaviour to pin.

Co-authored-by: Cursor <cursoragent@cursor.com>

* round-2 commit 13: retype IClusterEventBus.SubscribeAsync to speak ScopedSystemId?

Round-2 finding #24 of the second strong-typing rescan. The canvas rated
this LOW severity ("cost > value until we hit the bug") because it's an
interface change that has to thread through every subscriber, but committed
to it in the recommended-refactor sequence because the asymmetry is a real
bug vector: pre-Round-2 the bus's `Subscription.TargetSystemId` was
`SystemId?` and the publisher-side `ITargetedClusterEvent.TargetSystemId`
was `ScopedSystemId`, and `InProcessEventBus.PublishAsync` reconciled the
two by normalising both sides through `SystemIdNormalization.StripRegionPrefix`
on every publish tick. Any subscriber that ever supplied a scoped-shaped
`SystemId` (e.g. via `ScopedSystemId.AsSystemId`) without also stripping
would silently miss every delivery.

Wire changes:

* `IClusterEventBus.SubscribeAsync<TEvent>(SystemId? targetSystemId, ...)`
  becomes `SubscribeAsync<TEvent>(ScopedSystemId? targetSystemId, ...)`.
  The parameterless broadcast overload (default-interface-method that
  delegates with `targetSystemId: null`) is unchanged.
* `InProcessEventBus.Subscription<TEvent>.TargetSystemId` promoted from
  `SystemId?` to `ScopedSystemId?`, matching the wire shape of every
  `ITargetedClusterEvent.TargetSystemId`.
* `InProcessEventBus.PublishAsync` filter simplified from the two-sided
  strip-then-string-equals shape to a scoped-record-struct-equality
  compare. Byte-equivalent to the pre-Round-2 shape when both regions
  match; correctly rejects the cross-region false positive that the
  strip-then-compare shape used to deliver (canvas #24's stated bug
  vector).

Socket subscription flow (the load-bearing subscriber):

* `WebSocketHandler.IsSocketJoinTokenAuthorizedAsync` return tuple gained a
  third element `ScopedSystemId? TokenSubject`. The parse already happened
  inside the helper at the `TryParseScoped` gate (Step 5 of the first
  strong-typing rescan), so returning it is free — the caller no longer
  has to re-derive the scoped composite from either the raw topic id (which
  would need a region resolver) or by re-parsing the JWT.
* `SocketPushContext` gained a `JoinedScopedSystemId` (`ScopedSystemId?`)
  alongside the pre-existing raw `JoinedSystemId` (which is retained
  because every socket event handler routes through
  `TryGetSystemTopic(SystemId)` and those signatures don't change here).
* `SocketEventPumpRunner.SubscribeAsync` now feeds
  `context.JoinedScopedSystemId` to the bus. The pump's ~38 subscriptions
  now filter on scoped-to-scoped, matching the wire shape end-to-end.
* `WebSocketHandler.HandleAsync` destructures the three-tuple and threads
  the parsed `scopedSub` into the `SocketPushContext` constructor.

Tests:

* `WireByteFreezeTests.InProcessEventBus_RoutesScopedAndRawInterchangeably`
  was renamed to `InProcessEventBus_ScopedToScopedMatchDelivers` and
  rewritten to reflect the new invariant: subscriber and publisher both
  speak `ScopedSystemId`, and the test pins the load-bearing single-region
  happy path (which is exercised by every WebSocket push in the codebase).
  The pre-Round-2 shape asserted that a raw-subscription received scoped
  events via `StripRegionPrefix` tolerance; that tolerance is gone by
  design and can't be exercised because the interface no longer accepts a
  raw `SystemId` subscription target.
* New test: `InProcessEventBus_CrossRegionScopedTargetsDoNotBleedAcross`
  pins that a NAM-scoped subscriber does NOT receive an EUR-scoped publish
  even when the raw ids match — the pre-Round-2 strip-then-compare shape
  WOULD have delivered this cross-region false positive, so the new
  scoped-to-scoped compare is not only a type-safety win but also a real
  correctness fix.
* `ImportJobBackgroundServiceTests` in-file test stub updated to the new
  interface signature (only touchpoint outside the socket flow — the other
  test stubs and integration-test fixtures all subscribe with
  `targetSystemId: null` via the parameterless broadcast overload, which
  is unchanged).

Cross-region regression pin: the new
`InProcessEventBus_CrossRegionScopedTargetsDoNotBleedAcross` test would
have failed against the pre-Round-2 bus body (it delivered the eur-target
event to the nam-subscriber because both normalised to the same raw id).
The passing behaviour is the correctness improvement the canvas finding
identified.

Build: 100 warnings (unchanged from Commit 12 baseline), 0 errors.
Api.UnitTests: 229 passed (+1 for the new cross-region test), 0 failed.
Integration InProcessEventBusReconnectTests: 2 passed, 0 failed.

Co-authored-by: Cursor <cursoragent@cursor.com>

* round-3 commit 1: retype InMemoryStorageKeys.ForSystem to ScopedSystemId + cascade

Finishes Round-2 Commit 12's work by retyping the key side of every per-system
dictionary in the seven InMemory repositories. The pre-Round-3 InMemoryStorageKeys.ForSystem
returned string (immediately unwrapping the ScopedSystemId it just composed), which forced
every consumer to declare ConcurrentDictionary<string, TValue>. Round-2 Commit 12
tightened the value side of InMemoryAccountRepository but explicitly deferred the key-side
retype because it was cross-cutting. This commit closes that gap.

Round-3 canvas findings #1 + #2 + #3, landed as a single wave:

- #1: InMemoryStorageKeys.ForSystem returns ScopedSystemId. Seven repo-level
  GetSystemKey helpers now return ScopedSystemId. ~24 ConcurrentDictionary<string, ...>
  declarations across InMemoryAccountRepository / InMemoryAlterRepository /
  InMemoryFrontingRepository / InMemoryJournalRepository / InMemoryPollRepository /
  InMemorySettingsFieldRepository / InMemoryTagRepository retyped to
  ConcurrentDictionary<ScopedSystemId, ...>. Byte-identical lookup semantics — the
  record-struct default equality is ordinal on Value.

- #2: InMemoryAlterRepository.AlterState.Fields promoted from
  Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase) to
  Dictionary<FieldId, string?>. Every field id in the codebase is
  Guid.NewGuid().ToString("N") (lowercase hex) so the OrdinalIgnoreCase behaviour is
  vacuous — no observable drift.

- #3: InMemoryTagRepository._alterMemberships and InMemoryJournalRepository._entryAlters
  keys promoted from hand-concatenated "{systemKey}:{childId}" strings to typed
  (ScopedSystemId, TagId) / (ScopedSystemId, EntryId) ValueTuples. Structural equality
  for free; the concatenated composite is gone.

Side-effect fix: InMemoryAccountRepository.FindOrCreateSystemIdByDiscordIdAsync /
FindSystemIdByEmailAsync / FindSystemIdByAppleIdAsync had a latent shape mismatch pre-
Round-3 — the write side put the identity value under the RAW newSystemId key while every
downstream reader (LinkIdentifier, Unlink*, GetPublicProfile) looked up by the SCOPED
composite via GetSystemKey. The raw-string dictionary silently accepted both spellings so
the drift was invisible; the retype forces the write side to match the read side and
uses scopedNew as the key.

Verified: `dotnet build Interfold.slnx` clean (0 errors, 100 warnings unchanged);
229/229 Api.UnitTests pass; 374/375 Bootstrapper.UnitTests pass (1 skipped unix-only).

Co-authored-by: Cursor <cursoragent@cursor.com>

* round-3 commit 2: extract UserRegistryLookupColumn enum inside ScyllaUserRegistryRegionContext

Round-3 canvas finding #4. Same shape as Round-2 Commit 11's ProviderColumn extraction
inside ScyllaAccountRepository: the pre-Round-3 LookupAsync had a 5-branch
LookupKind switch dispatching to three magic-string CQL column names ("username",
"discord_id", "user_id"). A new lookup kind added later could easily drift from the
SQL builder if a maintainer forgot to update both sides.

Replace the (column, value) tuple destination with a typed
(UserRegistryLookupColumn, string). The enum is private-nested and reachable only
through a private-static ColumnName helper that produces the on-disk CQL column name at
exactly one call site. The CQL vocabulary stays encapsulated inside this class — no
extension method, no assembly-visible enum, no extra top-level type.

Zero behavioural change: byte-identical CQL statements, byte-identical cache keys,
byte-identical fallback path for unparseable inputs.

Verified: `dotnet build Interfold.slnx` clean (0 errors, 100 warnings unchanged);
229/229 Api.UnitTests pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* round-4: three residual strong-typing fixes missed by rounds 2 and 3

Round-4 audit refuted the Round-3 "next honest scan returns an empty page" claim
with three real findings. All three ship here as a single small commit — two of them
are the same class of bug already fixed elsewhere in earlier rounds that were missed
during the wide sweep.

Round-4 canvas findings #1 + #2 + #3:

- #1 (MEDIUM · Api): PublicSystemsController.Batch self-check promoted from the
  pre-R2C8 `principalId.RawId == SystemIdNormalization.StripRegionPrefix(systemId)`
  raw-byte-compare to `principalId.RepresentsSameUserAs(systemId)`. The old spelling
  stripped both sides and byte-compared, which falsely self-rejects cross-region
  requests where the viewer's `"eur:abcdefg"` asks for `"nam:abcdefg"` (different
  user, same raw id, distinct scoped composite). Same substitution R2C8 applied to
  8 sites across FriendsController / FriendRequestsController.

- #2 (MEDIUM · InMemory Tag + Fronting): ResolveFriendshipLevelAsync helpers in
  InMemoryTagRepository and InMemoryFrontingRepository were doing
  `systemId == viewerSystemId` — byte-compare on .Value. The correct reference
  implementation lives in InMemoryAlterRepository (which strips region prefixes on
  both sides) and the Scylla port has been consolidated onto
  ScyllaSharedQueries.ResolveFriendshipLevelAsync since R2C6 (which does the
  equivalent normalization). Tag/Fronting drift meant a viewer arriving scoped
  ("nam:abcdefg") against an owner arriving raw ("abcdefg") would miss the
  self-branch and degrade the owner's own-system visibility — the same drift-shape
  R3C1 fixed for account lookups. Both helpers now match Alter/Scylla exactly.

- #3 (LOW · Contracts + Api + InMemory): EncryptionSalt.NewRandom() factory added
  next to EncryptionSalt in Interfold.Contracts/Ids/EncryptionMaterial.cs; adopted
  at AuthController.SetupEncryption and InMemoryAccountRepository.EnsureEncryptionSaltForSystem.
  Same shape as R2C4's Jti.NewJti(): the raw base64 string spends zero time as a
  bare local (EncryptionSalt.ToString redacts, string.ToString does not) and the
  32-byte width lives in the wrapper rather than being hard-coded at each mint site.
  Removes the now-unused `using System.Security.Cryptography` from AuthController.

Verified: `dotnet build Interfold.slnx` clean (0 errors, 100 warnings unchanged);
229/229 Api.UnitTests pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Round-5 sanity check: post-Round-4 dead-code sweep

Delete three surfaces that Rounds 1–4 orphaned. All grep-verified zero live
callers before deletion. Skipping the fourth candidate (`ExecuteCommandAsync`
on the controller base) — kept as scaffold for a later metrics-migration PR.

Findings:

1. `SocketPushContext.JoinedSystemId` (property + ctor param + `WebSocketHandler`
   pass site). R2C13 introduced `JoinedScopedSystemId` and rewired the pump's
   `SubscribeAsync` filter over to it. The XML doc preserved the raw sibling
   "so socket event handlers don't have to change signatures" — but the
   handlers route on `evt.TargetSystemId` fed into `TryGetSystemTopic(SystemId)`
   and never read `context.JoinedSystemId`. Only remaining references were the
   `WebSocketHandler` ctor pass site and comments in `SocketEventPumpRunner`.
   Comments in both files updated to name `JoinedScopedSystemId` instead. The
   `WebSocketHandler` `joinedSystemId` local stays (still needed for
   `BuildJoinInitPayloadAsync` and `HandleEndpointProxyAsync`).

2. `InterfoldControllerBase.QualifyUrl(string?)`. R2C2 typed every avatar
   read-model field to `AvatarUrl?` and R2C10 deleted the untyped
   `QualifyAvatar(string?, AvatarSource?)` overload. Every avatar consumer
   flows through the typed `QualifyAvatar(AvatarUrl?, AvatarSource?)` today.
   Exactly two grep occurrences of `QualifyUrl` in the solution — both the
   declaration itself. Non-avatar callers wanting origin qualification without
   an `AvatarSource` discriminator can call `AvatarUrlQualifier.Qualify`
   directly (that's the shim `QualifyUrl` used to delegate to anyway).

3. Both `InMemoryStorageKeys.NormalizeSystemId` overloads
   (`(SystemId) → string` and `(string) → string`). R3C1 introduced
   `Normalize(SystemId) → SystemId` so the InMemory repos could key their
   dictionaries on the typed wrapper directly. Every InMemory repo migrated
   to the typed version. Grep-verified zero references to either overload
   across the whole solution — pure migration residue. `SystemIdNormalization`
   still calls itself a "public shim" and has 6 live call sites; that seam
   is legitimately still in use.

Verified: `dotnet build .\Interfold.slnx` clean at 0 errors, 100 warnings
(unchanged from post-Round-4 baseline). `Interfold.Api.UnitTests` 229/229 pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Round-5 sanity check: consolidate InMemory ResolveFriendshipLevelAsync

Finish the Round-2-Commit-6 story on the InMemory port. R2C6 collapsed the
three Scylla `ResolveFriendshipLevelAsync` wrappers onto
`ScyllaSharedQueries.ResolveFriendshipLevelAsync`, leaving the same three-way
duplication on the InMemory side. Round-4 finding #2 fixed the Tag and Fronting
copies to match Alter's reference implementation — that alignment is exactly
what makes the collapse safe now.

Add `InMemoryStorageKeys.ResolveFriendshipLevelAsync(SystemId, SystemId?,
IFriendshipRepository?, CancellationToken)` as the single source of truth for
the InMemory port. The three per-repo private helpers (Alter:369, Tag:281,
Fronting:325) become one-line delegators that thread each repo's nullable
`_friendships` field through the shared static. All five internal call sites
across the three repos remain unchanged — encapsulation of `_friendships` is
preserved.

The R4 attribution comment (StripRegionPrefix self-check on both sides vs the
pre-R4 byte compare that silently missed scoped-vs-raw drift between owner
and viewer) now lives on the shared static's XML doc as the single source of
truth for the invariant.

Verified: `dotnet build .\Interfold.slnx` clean at 0 errors, 100 warnings
(unchanged). `Interfold.Api.UnitTests` 229/229 pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Round-5 sanity check: collapse IAccountRepository provider fan-out

Finding 6. Replace the six per-provider public methods on IAccountRepository
with two ProviderIdentity-keyed methods that dispatch inside the repo instead
of at the call site. The pre-Round-5 shape had two coupled footguns that only
became worth fixing after Rounds 2/3 landed:

  - The fan-out (FindOrCreateSystemIdByDiscordIdAsync +
    FindSystemIdByEmailAsync + FindSystemIdByAppleIdAsync + the three
    matching Link* methods) mirrored the same 3-branch pattern-match in
    every caller. Both AuthController.Callback and AuthLinkController.Callback
    did an identical `typedIdentity switch { { Discord: ... } => ...Discord...,
    { Google: ... } => ...Email..., { Apple: ... } => ...Apple..., _ => ... }`.
    R2C7's ProviderIdentity union made the union available; R2C11's
    ProviderColumn made the Scylla internal dispatch typed. This commit
    finishes that story at the interface itself.

  - "FindSystemIdByEmailAsync" and "FindSystemIdByAppleIdAsync" advertised
    a "find" verb but their implementations were create-on-miss — only the
    Discord method's name (FindOrCreate...) reflected auto-provisioning
    truthfully. Consolidating onto FindOrCreateSystemIdAsync honours the
    invariant for all three providers and matches the create-on-miss
    behaviour the OAuth-login flow always wanted.

TryFindSystemIdByDiscordIdAsync stays. It's the only surviving identity-
specific find method — its Email / Apple siblings never had callers because
the OAuth-login flow always auto-provisions on miss and the friendship
resolver only supports the Discord shape. The XML doc now spells out why it
lives alongside the consolidated FindOrCreateSystemIdAsync rather than
folding in as an "auto-provision?" flag on the union.

Scope:

  - IAccountRepository: 6 methods → 2 (FindOrCreateSystemIdAsync +
    LinkIdentityToUserAsync), keeping the six existing Unlink* + delete
    + profile methods unchanged.
  - ScyllaAccountRepository: 6 line-per-method delegations collapse into 2
    ProviderIdentity switch expressions that dispatch into the same
    ProviderColumn-typed internal helpers R2C11 already established.
  - InMemoryAccountRepository: FindOrCreateSystemIdAsync dispatches to three
    private per-branch helpers (each still writes to its distinct pair of
    identity/reverse-map dicts, so the write-side logic stays per-provider).
    LinkIdentityToUserAsync dispatches via the pre-existing generic
    LinkIdentifier helper.
  - AuthController.Callback: 3-branch switch → single non-dispatching call.
  - AuthLinkController.Callback: 3-branch switch → single non-dispatching call.
  - ResolveUserIdDispatchTests.RecordingAccountRepository: 6 NotSupportedException
    stubs → 2 (matches the interface shape). Updated the two decision-comment
    blocks that referenced the old FindOrCreate* / Find* names.
  - SendFriendRequestPrefixTests + ScyllaFriendshipRepository decision-comment
    updates to name the new consolidated method.

Verified: dotnet build .\Interfold.slnx clean at 0 errors, 100 warnings
(unchanged from post-Round-4 baseline). Interfold.Api.UnitTests 229/229 pass
(the load-bearing ResolveUserId_DiscordPrefix_UnknownId_ReturnsNull_NoPhantomCreate
pin still holds — it exercises TryFind and the auto-provisioning consolidation
is legibly separate).

Co-authored-by: Cursor <cursoragent@cursor.com>

* Round-5 sanity check: consolidate ResolveMultipartUploadAsync

Finding 7. Extract the byte-identical 59-line multipart-upload helper from
AltersController and SettingsController onto InterfoldControllerBase. Both
controllers inherit from the base already, so the two internal call sites
(AltersController.UploadAvatar and SettingsController.UploadAvatar) bind
unchanged to the newly-inherited protected method.

Honesty note: this is not a typing-unlocked refactor. The duplicated helper
predates Round 1 entirely — its shape reads from HttpRequest.Body and returns
an AvatarUploadPayload, neither of which was touched by the four rounds of
strong-typing work. Included in Round 5 because Category 2 of the sanity
check ("N-copy helpers that can now share one implementation") explicitly
covers cloned controller helpers, and this is the largest verbatim controller
clone still standing in the API layer. Flagged as such in the extracted
method's XML doc so a future reader understands why it landed in this batch
rather than in a strong-typing commit.

Also trims the four `using`s that were only there for the deleted helper's
call surface (MultipartReader / MediaTypeHeaderValue / etc.) — two
per-controller. Net dedup: ~60 lines removed from the two controllers, ~90
added on the base (helper + full XML doc explaining the fold), so line count
is roughly break-even but the drift-risk surface halves.

Verified: dotnet build .\Interfold.slnx clean at 0 errors, 100 warnings
(unchanged from post-Round-4 baseline). Interfold.Api.UnitTests 229/229 pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Round-5 sanity check: inline FriendshipSocketEventHandlers.IdMatches

Finding 8. Delete the two-line `private static bool IdMatches(string? left,
string right) => SystemTopic.IdMatches(left, right)` pass-through in
FriendshipSocketEventHandlers and inline the single call site to call
SystemTopic.IdMatches directly. The private wrapper added no behaviour beyond
narrowing the second parameter's nullability annotation — pure residue from
the Round-1 socket-token typing work that never earned its keep as a distinct
symbol.

Verified: dotnet build .\Interfold.slnx clean at 0 errors, 100 warnings
(unchanged). Interfold.Api.UnitTests 229/229 pass.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Simplify code comments across the codebase

Strip internal audit terminology (round / slice / commit / finding / canvas
references) and condense verbose commentary into terse, purposeful comments
that focus on the current invariants, contracts, and rationale readers need.
No production or test behaviour changes; XML docs and inline comments only.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Refactor Scylla repositories to use string for normalizedSystemId

Co-authored-by: Cursor <cursoragent@cursor.com>

* refactor(wrappers): adopt target-typed new() and implicit-widen at call sites
The id/token wrappers in Interfold.Contracts.Ids ship explicit narrow
(`(W)raw`) and — for non-PII wrappers — implicit widen (`W -> primitive`)
operators. This adopts them at ~80 sites across Interfold.Domain, .SPDump,
.Infrastructure.InMemory, .Api, .Api.UnitTests, and .IntegrationTests:
- `new W(x)` -> target-typed `new(x)` at typed sinks (variable declarations,
  returns, arguments, initializers). `var` bindings are spelled out
  (`SystemId x = new(raw);`) rather than falling back to `(W)raw`.
- `.Value` dropped at string sinks where the widen is direct:
  IsNullOrWhiteSpace, TryParseScoped, StripRegionPrefix, IdMatches, dict
  lookups, `$"..."` interpolation, and StringComparer.Ordinal-anchored
  OrderBy keys via `(string)w`.
PII-redacting wrappers (DiscordId, Email, AppleId, LinkToken, PushToken,
ImportToken, RecoveryCode, Jti, SocketToken, EncryptionKeyMaterial,
KeyChecksum, EncryptionSalt) intentionally have no implicit widen — dropping
`.Value` on those would silently defeat SecretRedaction under any
`Foo(string)` overload. Construction still adopts `new()`, but every unwrap
keeps `.Value`. Guarded by PiiWrapperImplicitWidenAbsenceTests.

* Adopt wrapper conversion operators across Scylla infrastructure

* Back TagId/PollId/EntryId/FrontId/FieldId with Guid instead of string

Wrappers now hold a Guid directly; route binding and JsonConverter reject
non-uuid input at the boundary via UuidString.TryParse, and the wire form
(compact 32-char hex) is preserved so idempotency hashes and response
bodies stay byte-identical. Deletes every per-repo TryParseUuid guard
across Scylla + InMemory, collapses the FieldId UDT-join in
ScyllaSharedQueries to a direct Guid compare, and swaps
Dictionary<string, string?>/Guid.Parse for Dictionary<Guid, string?> in
ScyllaAlterRepository. Boundary consumers that used
string.IsNullOrWhiteSpace on the wrapper switch to Guid.Empty checks.
Sort keys made explicit (.Value.ToString("N")) to keep list ordering
stable.

* Centralize Enum handling and use the built in JsonStringEnumConverter

* [Skip CI] reword some comments

* [Skip CI]
- Removed namespace
- Inlined consts where it made sense.
- Use Mocking in place of some stubs

* Tighten friend-request wire contract to id/username only. Move and rename LookupHandle to UserRegistryLookup for internal registry routing. Update docs and comments to clarify contract and rationale.

* Bind Alter Id check within models, to add for struct itself in a second pass

* Move remaining alter id checks to new logic

* Remove BaseRequest

* Refactor system ID normalization to use ScopedSystemId. Removed SystemIdNormalization in favor of ScopedSystemId's StripRegionPrefix method across various files, ensuring a single source of truth for region prefix stripping. Updated related comments and documentation for clarity.

* Many self code review changes

* BInd UnixSeconds and fix tests failing

* Refactor SimplyPluralImportService color parsing to use HexColor normalization. Introduce TolerantWireEnumJsonConverter for nullable enum handling in PhxJoinPayload, ensuring unknown platform values round-trip to null without exceptions. Add unit tests for AlterId and HexColor to enforce strict validation and JSON serialization behavior.

* Fix remaining unit test issues

* Last set of reviews for this PR

* Don't drop & rename, we are unable to because of the Syclla limits...

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Azyyyyyy referenced this pull request in Azyyyyyy/Interfold Jul 21, 2026
- #18 / #19 InMemoryFrontingRepository — drop the nullable-and-defaulted
  ctor parameters on `_friendships` / `_alters` and their fields. DI wires
  both (InMemoryServiceCollectionExtensions.AddInMemoryPersistence) so the
  nullability wasn't buying anything and the two guard branches in
  ListActiveAsync / ListActiveGuardedAsync would silently hide a DI mis-
  wiring instead of surfacing it. Collapses the "have `_alters` or fall
  back to a placeholder" branch back to a single expression.

- #24 ScyllaSharedQueries.ResolveAlterFields — swap the FirstOrDefault
  scan for a Dictionary<Guid, string?> keyed by field id so the per-
  definition projection is O(N + M) instead of O(N * M). Same visible
  behaviour on the wire; a system with a handful of alters and a handful
  of settings-field definitions no longer allocates a fresh iterator per
  definition. Also reverts the manual `def.Id.Value` unwrap to `def.Id`
  and lets FieldId's implicit-to-Guid operator do the work — matches
  wrapper-typed comparison intent from the review.

- #15 New AlterCommandFlowExecutionTests — three targeted tests that pin
  UpdateAlterCommandHandler's reject / mutation-failed / happy-path
  branches through the same call-count assertion style as
  PollCommandFlowExecutionTests. The MissingAlter case is the specific
  coverage gap the review flagged; the other two guard against a future
  refactor accidentally catching the accept path in the reject branch.

Build clean (0 errors). New tests: 3 pass. Neighbour suites (Front*, Alter*)
all pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Discord This is regarding the Octocon Discord bot. Enhancement Implementation of a new feature. High Priority This is a high priority PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discord autocomplete for friends & friend requests Slash command autocomplete

1 participant